Skip to content

fix(content-sharing): grey link expiration from server flag - #4886

Open
jpan-box wants to merge 2 commits into
box:masterfrom
jpan-box:fix-grey-shared-link-expiration
Open

jpan-box wants to merge 2 commits into
box:masterfrom
jpan-box:fix-grey-shared-link-expiration

Conversation

@jpan-box

@jpan-box jpan-box commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

MOTIVATION

Desktop Content Sharing turned Link Expiration on when the user could change the shared link access level and an editor role was allowed. GET /2.0/files/{id}?fields=shared_link_features now returns expiration for that permission. The switch follows that value.

BEFORE

  • Link Expiration stays on when the user can change share access and an editor role is allowed
  • A files response with no expiration value uses that same check

AFTER

  • Link Expiration stays on when shared_link_features.expiration is true
  • A files response with no expiration value leaves the switch off

Summary by CodeRabbit

  • Bug Fixes
    • Shared-link expiration settings now follow the expiration capability provided by the server, including when access permissions are restricted.

@jpan-box
jpan-box requested a review from a team as a code owner October 6, 2026 16:20
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5fecee68-6c24-4aac-adfe-18982c8086a0
📥 Commits

Reviewing files that changed from the base of the PR and between 7d71272 and 7d9ccad.

📒 Files selected for processing (5)
  • src/common/types/core.js
  • src/elements/content-sharing/types.js
  • src/elements/content-sharing/utils/__mocks__/ContentSharingV2Mocks.js
  • src/elements/content-sharing/utils/__tests__/convertItemResponse.test.ts
  • src/elements/content-sharing/utils/convertItemResponse.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


Walkthrough

Shared-link response types and the default mock now include an expiration capability. Item conversion uses the server-provided capability to set canChangeExpiration. Tests cover both capability values.

Changes

Shared-link expiration capability

Layer / File(s) Summary
Expiration capability and conversion
src/common/types/core.js, src/elements/content-sharing/types.js, src/elements/content-sharing/utils/convertItemResponse.ts, src/elements/content-sharing/utils/__mocks__/ContentSharingV2Mocks.js, src/elements/content-sharing/utils/__tests__/convertItemResponse.test.ts
The shared-link types and default mock include the required expiration boolean. Conversion uses the server-provided value for canChangeExpiration instead of checking share-access permission and editor role. Tests cover both values and update related fixtures.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: reneshen0328

Merge Risk: ⚪ Minimal · up to 7d9cc

No actionable merge-blocking risk remains in the reviewed changes.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the Content Sharing link expiration change and states that it now uses the server flag.
Description check ✅ Passed The description explains the motivation and compares the behavior before and after the change. It covers the server-provided expiration flag and the absent-field behavior.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 5…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 Biome (2.5.14)
src/common/types/core.js

File contains syntax errors that prevent linting: Line 126: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 476: 'export type' declarations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 37: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 39: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 40: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 41: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 43: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 50: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 51: type alias are a

... [truncated 6627 characters] ...

onvert your file to a TypeScript file or remove the syntax.; Line 447: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 455: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 459: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 463: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 465: Expected a statement but instead found ',
}'.; Line 470: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 128: type alias are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.

src/elements/content-sharing/types.js

File contains syntax errors that prevent linting: Line 7: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 156: Expected a semicolon or an implicit semicolon after a statement, but found none; Line 158: Expected a type but instead found '?'.; Line 161: Expected a type but instead found '?'.; Line 161: Expected a property, or a signature but instead found ','.; Line 187: Expected a type but instead found '?'.; Line 187: Expected a property, or a signature but instead found ';'.; Line 2: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 6: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 193: Expected a statement but instead found '}'.; Line 18: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 27: type alias are a TypeScript only feature. Co

... [truncated 948 characters] ...

pected an expression but instead found '}'.; Line 132: ';' expected'; Line 132: Expected a property, or a signature but instead found ','.; Line 135: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 137: Expected a semicolon or an implicit semicolon after a statement, but found none; Line 139: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 143: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 147: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 149: Expected a semicolon or an implicit semicolon after a statement, but found none


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the link at night
The server flag shines clear and bright
True lets expiration change
False keeps it out of range
The tests hop through each state
And leave the types up to date

Comment @coderabbitai help to get the list of available commands.

@jpan-box
jpan-box marked this pull request as draft October 6, 2026 22:31
@jpan-box
jpan-box force-pushed the fix-grey-shared-link-expiration branch from 7d71272 to fa7f945 Compare October 6, 2026 22:32
Comment thread src/common/types/core.js Outdated

type SharedLinkFeatures = {
download_url: boolean,
expiration?: boolean,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't expiration always exist now since we have added the field in the response

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yeah you're right

Comment thread src/elements/content-sharing/types.js Outdated
shared_link?: APISharedLink,
shared_link_features: {
download_url: boolean,
expiration?: boolean,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

@jpan-box
jpan-box marked this pull request as ready for review October 7, 2026 19:21

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants